Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(router): redesign JWK authentication logic #1498

Merged
merged 14 commits into from
Jan 30, 2025

Conversation

Noroth
Copy link
Contributor

@Noroth Noroth commented Jan 8, 2025

Motivation and Context

The current implementation for the authentication logic is not optimal. We are creating a token decoder per JWKs while the intention for the library is to have multiple sources where a keyset can be loaded and refreshed from.

This PR introduces a redesign of the current implementation. The configuration changes from a per JWKS basis to a global config with some options per JWKs (e.g. whitelisting algorithms when loading keys)

The token logic has been simplified to use a single token decoder, that can handle all keys and header/valueprefix combinations.

Warning

As part of the preparations for Cosmo V1, targeted for release in Q1 2025, this pull request introduces essential changes to enhance long-term stability and maintainability. While we strive to minimize breaking changes, they are sometimes necessary to lay the foundation for a more robust and scalable system.

Config change example:

Old New
authentication:
  providers:
    - name: "jwk1"
      jwks:
        refresh_interval: 1m
        url: "https://test1.io.well-known/jwks.json"
        header_value_prefixes:
          - "Bearer"
          - "Token"
          - "MyBearer"
        header_names:
          - "Authorization"
          - "X-Auth-Token"
    - name: "jwk2"
      jwks:
        refresh_interval: 2m
        url: "https://test2.io/.well-known/jwks.json"
        header_value_prefixes:
          - "Bearer"
          - "Token"
          - "MyBearer"
        header_names:
          - "Authorization"
          - "X-Auth-Token"
          - "X-Authorization"
authentication:
  jwt:
    jwks:
      - url: "https://test1.io/.well-known/jwks.json"
        refresh_interval: 1m
        # Leaving algorithms empty will allow all supported
        # algorithms from the config docs
      - url: "https://test2.io/.well-known/jwks.json"
        refresh_interval: 2m
        # optional list of allowed algorithms per JWKS
        algorithms: ["RS256", "EdDSA", "HS512"]
    header_name: Authorization # This is the default value
    header_value_prefix: Bearer # This is the default value
    # additional header/prefix combinations
    header_sources:
      - type: header 
        name: X-Auth-Token
        value_prefixes: [Token, MyToken]
      - type: header
        name: X-Authorization

Checklist

  • I have discussed my proposed changes in an issue and have received approval to proceed.
  • I have followed the coding standards of the project.
  • Tests or benchmarks have been added or updated.
  • Documentation has been updated on https://github.com/wundergraph/cosmo-docs.
  • I have read the Contributors Guide.

@github-actions github-actions bot added the router label Jan 8, 2025
Copy link

github-actions bot commented Jan 9, 2025

Router image scan passed

✅ No security vulnerabilities found in image:

ghcr.io/wundergraph/cosmo/router:sha-8e78e803d5cbf7be2786774dad4cffbaaa3d970f

router/pkg/config/config.go Outdated Show resolved Hide resolved
@Noroth Noroth force-pushed the ludwig/eng-6230-redesign-jwk-authentication-logic branch from cc46adb to eadd615 Compare January 14, 2025 14:08
@Noroth Noroth force-pushed the ludwig/eng-6230-redesign-jwk-authentication-logic branch from b924099 to d9d341b Compare January 21, 2025 09:41
@Noroth Noroth marked this pull request as ready for review January 21, 2025 12:39
Copy link
Contributor

@StarpTech StarpTech left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall great work. Just a few nuts.

router-tests/authentication_test.go Outdated Show resolved Hide resolved
router/pkg/authentication/validation_store.go Show resolved Hide resolved
router-tests/cmd/jwks-server/main.go Show resolved Hide resolved
@StarpTech
Copy link
Contributor

Please document how the migration will look like and mention that it is breaking.

Copy link
Contributor

@StarpTech StarpTech left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Noroth
Copy link
Contributor Author

Noroth commented Jan 21, 2025

I will prepare the documentation changes before merging this PR

@Noroth Noroth merged commit ecdf74a into main Jan 30, 2025
13 checks passed
@Noroth Noroth deleted the ludwig/eng-6230-redesign-jwk-authentication-logic branch January 30, 2025 12:14
james-braund-cabiri added a commit to cabiri-io/cosmo that referenced this pull request Jan 31, 2025
* feat: expose type data and record subgraphs for enums (wundergraph#1495)

* chore(release): Publish [skip ci]

 - [email protected]
 - [email protected]
 - @wundergraph/[email protected]
 - [email protected]
 - @wundergraph/[email protected]
 - [email protected]

* feat: improve rate limit responses (add code, hide stats) (wundergraph#1497)

* chore(release): Publish [skip ci]

 - [email protected]

* fix: provider should be specified in the config.yaml (wundergraph#1397)

* fix: update the timeouts for clickhouse and platform service (wundergraph#1500)

* chore(release): Publish [skip ci]

 - [email protected]
 - [email protected]
 - [email protected]

* fix: add edfs to the demo environment (wundergraph#1505)

* docs(CONTRIBUTING): fixup minor mistake in CONTRIBUTING.md under Go workspace (wundergraph#1502)

Co-authored-by: Dustin Deus <[email protected]>

* fix: full demo broken in main branch (wundergraph#1508)

* feat(router): optionally add jitter to config polling interval (wundergraph#1506)

Co-authored-by: Dustin Deus <[email protected]>

* chore(release): Publish [skip ci]

 - [email protected]

* fix(router): remove wildcard from router graphql path (wundergraph#1509)

* fix: use gauge for server.uptime metric (wundergraph#1510)

Co-authored-by: Ludwig <[email protected]>

* feat: cache warmer (wundergraph#1501)

Co-authored-by: Ludwig <[email protected]>
Co-authored-by: starptech <[email protected]>

* chore(release): Publish [skip ci]

 - [email protected]
 - @wundergraph/[email protected]
 - [email protected]
 - @wundergraph/[email protected]
 - [email protected]
 - [email protected]
 - [email protected]
 - @wundergraph/[email protected]
 - [email protected]

* fix(cache warmup): consider only po of the last 7 days (wundergraph#1513)

* chore(release): Publish [skip ci]

 - [email protected]

* fix(cache operation): swallow cache errors and other improvements (wundergraph#1515)

* chore(release): Publish [skip ci]

 - [email protected]
 - [email protected]
 - [email protected]
 - [email protected]

* feat: add variables remapping support (wundergraph#1516)

Co-authored-by: starptech <[email protected]>

* chore(release): Publish [skip ci]

 - [email protected]

* fix(router): write proper line endings and header for multipart (wundergraph#1517)

* chore(release): Publish [skip ci]

 - [email protected]

* feat(router): optimize playground delivery, add concurrency_limit to config (wundergraph#1519)

* fix(router): enable health checks during startup (wundergraph#1529)

* feat: improve cache warmer (wundergraph#1530)

Co-authored-by: Ludwig <[email protected]>

* chore(release): Publish [skip ci]

 - [email protected]
 - [email protected]
 - [email protected]

* fix: remove semaphore from ResolveGraphQLSubscription (wundergraph#1532)

* chore(release): Publish [skip ci]

 - [email protected]

* feat: add compatibility handshake between router and execution config (wundergraph#1534)

* chore(release): Publish [skip ci]

 - [email protected]
 - @wundergraph/[email protected]
 - @wundergraph/[email protected]
 - [email protected]
 - [email protected]
 - @wundergraph/[email protected]
 - [email protected]

* feat: also add handshake for static execution configs (wundergraph#1535)

* chore(router): bump demo library to pickup subscription fix (wundergraph#1518)

* feat(router): add interface for trace propagation (wundergraph#1526)

* chore(release): Publish [skip ci]

 - [email protected]

* fix: adding/removing directive is not picked up by wgc subgraph check (wundergraph#1494)

* chore(deps): upgrade ristretto to v2 (wundergraph#1538)

* feat: add normalizedQuery to query plan and request info to trace (wundergraph#1536)

Co-authored-by: df-wg <[email protected]>

* fix: add copy button to subgraph routing url (wundergraph#1543)

Co-authored-by: Dustin Deus <[email protected]>

* fix: webhooks shot when schema is unchanged (wundergraph#1542)

* fix: trim the inputs of group mappers (wundergraph#1541)

* fix: subgraphs search functionality (wundergraph#1540)

* chore(release): Publish [skip ci]

 - [email protected]
 - [email protected]
 - [email protected]
 - [email protected]

* fix: increase max concurrent resolvers (wundergraph#1544)

* refactor(router): redesign JWK authentication logic (wundergraph#1498)

* chore(release): Publish [skip ci]

 - [email protected]

* fix: increase the test timeout value to prevent failures on slower machines (wundergraph#1547)

* fix: reduce the breaking change retention duration (wundergraph#1550)

* fix: change the defaults of breaking-change-retention (wundergraph#1551)

* feat(router): enable starting the router without subgraphs (wundergraph#1533)

* fix(router): parse accept header per rfc 9110 (wundergraph#1549)

* chore(release): Publish [skip ci]

 - [email protected]
 - [email protected]
 - [email protected]

* feat(router): enable using redis clusters for rate limiting and apq (wundergraph#1499)

* fix: json schema for traffic shaping subgraphs (wundergraph#1552)

* chore: Update aws-lambda-router customisation after upstream sync

---------

Co-authored-by: Nithin Kumar B <[email protected]>
Co-authored-by: hardworker-bot <[email protected]>
Co-authored-by: Jens Neuse <[email protected]>
Co-authored-by: Alessandro Pagnin <[email protected]>
Co-authored-by: Suvij Surya <[email protected]>
Co-authored-by: endigma <[email protected]>
Co-authored-by: Dustin Deus <[email protected]>
Co-authored-by: Ludwig <[email protected]>
Co-authored-by: Sergiy 🇺🇦 <[email protected]>
Co-authored-by: df-wg <[email protected]>
Co-authored-by: Aenimus <[email protected]>
james-braund-cabiri added a commit to cabiri-io/cosmo that referenced this pull request Feb 4, 2025
* feat: expose type data and record subgraphs for enums (wundergraph#1495)

* chore(release): Publish [skip ci]

 - [email protected]
 - [email protected]
 - @wundergraph/[email protected]
 - [email protected]
 - @wundergraph/[email protected]
 - [email protected]

* feat: improve rate limit responses (add code, hide stats) (wundergraph#1497)

* chore(release): Publish [skip ci]

 - [email protected]

* fix: provider should be specified in the config.yaml (wundergraph#1397)

* fix: update the timeouts for clickhouse and platform service (wundergraph#1500)

* chore(release): Publish [skip ci]

 - [email protected]
 - [email protected]
 - [email protected]

* fix: add edfs to the demo environment (wundergraph#1505)

* docs(CONTRIBUTING): fixup minor mistake in CONTRIBUTING.md under Go workspace (wundergraph#1502)

Co-authored-by: Dustin Deus <[email protected]>

* fix: full demo broken in main branch (wundergraph#1508)

* feat(router): optionally add jitter to config polling interval (wundergraph#1506)

Co-authored-by: Dustin Deus <[email protected]>

* chore(release): Publish [skip ci]

 - [email protected]

* fix(router): remove wildcard from router graphql path (wundergraph#1509)

* fix: use gauge for server.uptime metric (wundergraph#1510)

Co-authored-by: Ludwig <[email protected]>

* feat: cache warmer (wundergraph#1501)

Co-authored-by: Ludwig <[email protected]>
Co-authored-by: starptech <[email protected]>

* chore(release): Publish [skip ci]

 - [email protected]
 - @wundergraph/[email protected]
 - [email protected]
 - @wundergraph/[email protected]
 - [email protected]
 - [email protected]
 - [email protected]
 - @wundergraph/[email protected]
 - [email protected]

* fix(cache warmup): consider only po of the last 7 days (wundergraph#1513)

* chore(release): Publish [skip ci]

 - [email protected]

* fix(cache operation): swallow cache errors and other improvements (wundergraph#1515)

* chore(release): Publish [skip ci]

 - [email protected]
 - [email protected]
 - [email protected]
 - [email protected]

* feat: add variables remapping support (wundergraph#1516)

Co-authored-by: starptech <[email protected]>

* chore(release): Publish [skip ci]

 - [email protected]

* fix(router): write proper line endings and header for multipart (wundergraph#1517)

* chore(release): Publish [skip ci]

 - [email protected]

* feat(router): optimize playground delivery, add concurrency_limit to config (wundergraph#1519)

* fix(router): enable health checks during startup (wundergraph#1529)

* feat: improve cache warmer (wundergraph#1530)

Co-authored-by: Ludwig <[email protected]>

* chore(release): Publish [skip ci]

 - [email protected]
 - [email protected]
 - [email protected]

* fix: remove semaphore from ResolveGraphQLSubscription (wundergraph#1532)

* chore(release): Publish [skip ci]

 - [email protected]

* feat: add compatibility handshake between router and execution config (wundergraph#1534)

* chore(release): Publish [skip ci]

 - [email protected]
 - @wundergraph/[email protected]
 - @wundergraph/[email protected]
 - [email protected]
 - [email protected]
 - @wundergraph/[email protected]
 - [email protected]

* feat: also add handshake for static execution configs (wundergraph#1535)

* chore(router): bump demo library to pickup subscription fix (wundergraph#1518)

* feat(router): add interface for trace propagation (wundergraph#1526)

* chore(release): Publish [skip ci]

 - [email protected]

* fix: adding/removing directive is not picked up by wgc subgraph check (wundergraph#1494)

* chore(deps): upgrade ristretto to v2 (wundergraph#1538)

* feat: add normalizedQuery to query plan and request info to trace (wundergraph#1536)

Co-authored-by: df-wg <[email protected]>

* fix: add copy button to subgraph routing url (wundergraph#1543)

Co-authored-by: Dustin Deus <[email protected]>

* fix: webhooks shot when schema is unchanged (wundergraph#1542)

* fix: trim the inputs of group mappers (wundergraph#1541)

* fix: subgraphs search functionality (wundergraph#1540)

* chore(release): Publish [skip ci]

 - [email protected]
 - [email protected]
 - [email protected]
 - [email protected]

* fix: increase max concurrent resolvers (wundergraph#1544)

* refactor(router): redesign JWK authentication logic (wundergraph#1498)

* chore(release): Publish [skip ci]

 - [email protected]

* fix: increase the test timeout value to prevent failures on slower machines (wundergraph#1547)

* fix: reduce the breaking change retention duration (wundergraph#1550)

* fix: change the defaults of breaking-change-retention (wundergraph#1551)

* feat(router): enable starting the router without subgraphs (wundergraph#1533)

* fix(router): parse accept header per rfc 9110 (wundergraph#1549)

* chore(release): Publish [skip ci]

 - [email protected]
 - [email protected]
 - [email protected]

* feat(router): enable using redis clusters for rate limiting and apq (wundergraph#1499)

* fix: json schema for traffic shaping subgraphs (wundergraph#1552)

* fix: subgraph timeout can't be bigger than global timeout (wundergraph#1548)

* fix: error when graph token is not set when cache warmup is enabled (wundergraph#1554)

* chore(release): Publish [skip ci]

 - [email protected]

* fix: incorrect graphql endpoint in playground (wundergraph#1562)

* chore(release): Publish [skip ci]

 - @wundergraph/[email protected]
 - [email protected]

* fix: update vulnerable packages (wundergraph#1560)

* fix: synchronize go mod versions (wundergraph#1564)

* chore: reduce verbose logging for failed tests (wundergraph#1565)

* fix: Add missing config mapping, bump aws-lambda-router version

* fix: Repair PNPM lockfile after merge

---------

Co-authored-by: Nithin Kumar B <[email protected]>
Co-authored-by: hardworker-bot <[email protected]>
Co-authored-by: Jens Neuse <[email protected]>
Co-authored-by: Alessandro Pagnin <[email protected]>
Co-authored-by: Suvij Surya <[email protected]>
Co-authored-by: endigma <[email protected]>
Co-authored-by: Dustin Deus <[email protected]>
Co-authored-by: Ludwig <[email protected]>
Co-authored-by: Sergiy 🇺🇦 <[email protected]>
Co-authored-by: df-wg <[email protected]>
Co-authored-by: Aenimus <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants